Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add computed def as function #31

Merged
merged 1 commit into from
Aug 28, 2015

Conversation

JSteunou
Copy link
Contributor

It might address #29 and be nice to have as it will allow to build the computed definition upon model context / options and facilitate inheritance

@alexbeletsky
Copy link
Owner

@JSteunou the code and tests looks good! but I'm trying to understand the practical meaning of this. can you give me example, where it's useful?

@JSteunou
Copy link
Contributor Author

First of all is inheritance. Like Model#defaults or most of Marionette hash attributes being able to be defined those as function allow this

computed: function() {
    var parentComputed = ParentModel.prototype.computed.apply(this, arguments);
    parentComputed.anotherFields = { ... };
    return parentComputed;
}

Second, it allows immutable definition.

Third, it could allow to give parameters to computed, or at least read model properties to return a dynamic computed hash context aware.

@alexbeletsky
Copy link
Owner

@JSteunou I would ask you to update README file with the information about that feature. Once it's there and since it doesn't brake existing behaviour, I see no reason to do not merge it :) Good job.

@JSteunou JSteunou force-pushed the feature/as-function branch from 1a6b4ac to 55e67ef Compare August 28, 2015 11:43
@JSteunou
Copy link
Contributor Author

README updated

alexbeletsky added a commit that referenced this pull request Aug 28, 2015
@alexbeletsky alexbeletsky merged commit be6f60b into alexbeletsky:master Aug 28, 2015
@JSteunou
Copy link
Contributor Author

Thank you, that was a fast PR :)

@JSteunou
Copy link
Contributor Author

Do you think a new release could follow? I would love to use this new feature :D

@alexbeletsky
Copy link
Owner

Will release it soon!
On Aug 28, 2015 4:13 PM, "Jérôme Steunou" [email protected] wrote:

Do you think a new release could follow? I would love to use this new
feature :D


Reply to this email directly or view it on GitHub
#31 (comment)
.

@alexbeletsky
Copy link
Owner

› npm publish
+ [email protected]

@JSteunou
Copy link
Contributor Author

Really nice, thank you!

@pebcakerror
Copy link

can you also tag the repo with 0.0.12 so that bower can use this feature

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants